Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[M2] app2 implementation #543

Merged
merged 44 commits into from
Sep 26, 2019
Merged

[M2] app2 implementation #543

merged 44 commits into from
Sep 26, 2019

Conversation

Darkren
Copy link

@Darkren Darkren commented Aug 30, 2019

Fixes #528

  • Added basic structures
  • Implemented methods for HSFrame

evanlinjin
evanlinjin reviewed Aug 30, 2019
View reviewed changes
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📄

pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
@evanlinjin evanlinjin changed the title [WIP] app2 implementation [M2] [WIP] app2 implementation Sep 4, 2019
evanlinjin
evanlinjin suggested changes Sep 9, 2019
View reviewed changes
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start.

pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Show resolved Hide resolved
pkg/app2/server.go Outdated Show resolved Hide resolved
evanlinjin
evanlinjin reviewed Sep 10, 2019
View reviewed changes
pkg/app2/listener.go Outdated Show resolved Hide resolved
pkg/app2/server.go Outdated Show resolved Hide resolved
evanlinjin
evanlinjin suggested changes Sep 18, 2019
View reviewed changes
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautifully written code.

Although there is some implementation queries. Specifically, we will need to support multiple networks in the future:

  • dmsg
  • skywire routes

My comments are mostly regarding this. Hence, you do not need to follow the exact comments, but please take some time to think of an optimal solution.

pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Show resolved Hide resolved
pkg/app2/manager.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
evanlinjin
evanlinjin suggested changes Sep 23, 2019
View reviewed changes
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean code and good work so far. Please review my comments.

pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Outdated Show resolved Hide resolved
pkg/app2/manager.go Outdated Show resolved Hide resolved
pkg/app2/network/dmsg_conn.go Outdated Show resolved Hide resolved
pkg/app2/rpc_gateway.go Outdated Show resolved Hide resolved
evanlinjin
evanlinjin suggested changes Sep 24, 2019
View reviewed changes
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff. Nearly there.

pkg/app2/network/addr.go Outdated Show resolved Hide resolved
pkg/app2/network/addr.go Outdated Show resolved Hide resolved
pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
@Darkren Darkren changed the title [M2] [WIP] app2 implementation [M2] app2 implementation Sep 26, 2019
@Darkren Darkren merged commit 0ff83f9 into skycoin:mainnet-milestone2 Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evanlinjin evanlinjin evanlinjin requested changes

Assignees
No one assigned
Labels
enhancement mainnet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Darkren @evanlinjin